Skip to content

[FEATURE] Data,Refinery: add QR code support#11391

Open
thibsy wants to merge 3 commits intoILIAS-eLearning:trunkfrom
srsolutionsag:feature/12/data-qr-code
Open

[FEATURE] Data,Refinery: add QR code support#11391
thibsy wants to merge 3 commits intoILIAS-eLearning:trunkfrom
srsolutionsag:feature/12/data-qr-code

Conversation

@thibsy
Copy link
Copy Markdown
Contributor

@thibsy thibsy commented Apr 2, 2026

Hi @mjansenDatabay,

This is the first iteration of my QR code abstraction for ILIAS, which I plan on using inside the UI framework to convert ILIAS\Data\URI objects into a QR code which can be embedded inline as a data URI.

I haven't worked out the exact details of the UI interface yet, so there may be changes in the meantime, but I'd still like to hear your 5 cents about this – it doesn't need to be an in-depth code review just yet. I will also wait with the JF label until you have approved of the details, although I will post the formal request in here some time soon already.

What I have worked out:

  • I have evaluated different QR code generation libraries, the one used here is the most low-level and widely used one, but details will come soon. Important to note is that all of them require two settings in addition to the string data which should be encoded: the error correction level and some kind of size, which is not always the same though (size in px vs. size for byte-map). Replacing the library would be very easy, since they all seem to mimic the ZXing library for Java.
  • I have determined that an SVG is the most suitable output format, since we do not create temporary files (although an underlying library might do so, but this one doesn't) and can always work with strings and in memory. It also allows us to easily embed this as a data URI. It also still allows us to create and store the QR code as files ourselves later on.
  • I also figured we need some DTO carrying the actual SVG data so it can be passed around between layers, in addition to some transformation that serves as the actual wrapper for the external library. Similar has been done for the markdown syntax.

What I haven't worked out (yet):

  • I am not yet sure how the accessibility of QR codes can be ensured and implemented centrally. I think Data and Refinery are not the most appropriate location to do so, since making the QR code accessible seems responsibility of the UI layer – be that the UI framework or not. That's why I did not introduce the "raw data" or some kind of "alternate text" attribute to the DTO, although I might change this in the future.
  • I was also kind of confused where to put the newly added objects (namespace and location), I figured for Data a new namespace made sense and for Refinery the String location is right. Let me know if you feel differently.
  • I would have liked to use DI for the new transformation inside the Refinery. However, this isn't done anywhere else and I wondered whether this is deliberately or not. Not doing so obviously makes the added unit tests highly dependant on the actual implementation details of the library in use. Let me know if I should wire this feature using the bootstrap mechanism.

Looking forward to your thoughts.

Thx and kind regards,
@thibsy

@thibsy thibsy added improvement dependencies Pull requests that update a dependency file php Pull requests that update Php code labels Apr 2, 2026
@thibsy thibsy force-pushed the feature/12/data-qr-code branch from 613b2fe to 3de0fa2 Compare April 2, 2026 13:24
* Add `ILIAS\Data\QR\SVGCode` wrapper for generated QR codes.
* Add `ILIAS\Data\QR\ErrorCorrectionLevel` for standardised error correction levels.
* Add `ILIAS\Refinery\String\QRCode` transformation to convert string into a QR code.
* Add unit tests covering the functionality above.
@thibsy thibsy force-pushed the feature/12/data-qr-code branch from 3de0fa2 to 0e88c35 Compare April 2, 2026 20:54
@thibsy
Copy link
Copy Markdown
Contributor Author

thibsy commented Apr 7, 2026

Assessment:

The bacon/bacon-qr-code is a lightweight, low-level QR code encoder that covers
all requirements for the planned QR code feature(s) in ILIAS (https://docu.ilias.de/go/wiki/wpage_8762_1357). It supports SVG output which allows us to work purely in memory, while still allowing us to store the image if need be.

General Information:

  • Name of the dependency: bacon/bacon-qr-code
  • Version: ^3.0
  • this dependency was already used in ILIAS.
  • the dependency's license is compatible with ILIAS' license. (BSD-2)

Type of dependency:

  • composer
  • npm

Usage:

  • ILIAS\Refinery\String\QRCode (refinery transformation)

Reasoning:

  • QR codes are a great medium for sharing URL addresses for various purposes. The mentioned feature request(s) all rely on the availability of a QR code abstraction which can be used to encode them.
  • We do not want to maintain a QR encoder ourselves, as this is very technical stuff and implementing some standard specification requires much more resources, of which we have so little at the moment.
  • The bacon/bacon-qr-code is the most popular low-level library there is, which is perfect for us because it provides minimal surface area. There are other widely used libraries which themselves rely on this one. Since we do not require feature-rich abstractions we can easily use this one.

Maintenance:

  • Maintained by multiple contributors, reducing the bus-factor risk
  • The repository is stable and evolves conservatively, which is appropriate for a low-level encoder library. Latest release was yesterday.
  • Widely used, both directly and as a transitive dependency, meaning issues are likely to surface and be addressed quickly by the more than just one community.

Links:

Alternatives:

There is only one other widely used library, which does not rely on the bacon/bacon-qr-code. Switching to this library would be very trivial, as the code for generating a QR code is very similar and it supports SVG as well. Most of the libraries in the wild seem to mimic the ZXing library for Java, which is something the LLMs are getting quite good at (porting stuff to different languages). Meaning, in the worst case we can simply ask one to port this for ILIAS ourselves.

@lscharmer
Copy link
Copy Markdown
Contributor

Hi @thibsy,
thank you for this PR.

here are my thoughts on this:
IMO the QRCode must be separate from the output encoding. It may be outputted as HTML, JSON or SVG but when passing a QRCode around in PHP itself we need the actual information that makes up a CRCode not what it will be outputted as.
Then we can provide a Transformation which transforms a CRCode to a SVG.
Creating a data URI from a SVG would be done with another transformation.

In terms of accessibility:
IMO the QR Code is only one way to present information in a convenient way to the user (please also note that this doesn't has to be a URI). So we actually want to pass around the information to the UI and not a QR Code. This way the UI can present the information in other ways as well. The QRCode will probably used only temporary between the UI layer and the transformation or maybe one could even drop the QRCode DTO and directly pass ErrorCorrection and Size information to the Transformation as the UI just wants a SVG which represents a QRCode.

Kind regards and thank you again for this PR
@lscharmer

@thibsy
Copy link
Copy Markdown
Contributor Author

thibsy commented Apr 9, 2026

Hi @lscharmer, thx for the quick feedback!

I think its not quite possible to pass around the information which makes up the QR-code, since this reaches into the dependency too far. We could only introduce a wrapper so we can limit what kind of data can be transformed into a QR-code result, but this wouldn't add much value. I'm therefore beginning to think we should just drop the DTO and only work in transformations. How does that sound to you? Give me a quick ping if we should discuss this on Discord.

Also thanks for your thoughts on the accessibility. You're totally right – this is exactly what my UI interface is starting to look like (encapsulate different formats of which QR code is only one).

Kind regards,
@thibsy

@thibsy
Copy link
Copy Markdown
Contributor Author

thibsy commented Apr 10, 2026

Hi @lscharmer,

As quickly discussed on Discord, I have removed the QR code DTO for the reasons above and implemented the abstraction primarily based on transformations. This PR now introduces:

  • a transformation from Data\URI to Data\SVG (new), and
  • a transformation from Data\SVG to string

Since the Data\URI does not support data URIs, the second transformation currently results in a string, which ought to be a Data\URI (or DataURI) some time. Thats why I implemented it all inside the Refinery\URI namespace – I hope thats fine.

Looking forward to a more concrete review now =).

Thx and kind regards,
@thibsy

@thibsy
Copy link
Copy Markdown
Contributor Author

thibsy commented Apr 10, 2026

P.S. once the changes are approved I will squash the commits appropriately, so we can rebase and integrate one commit for the abstraction (or two for each component if you prefer) and one for the dependency.

Copy link
Copy Markdown
Contributor

@lscharmer lscharmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @thibsy,
thanks updating this PR. The structure looks good to me, there is just one naming issue left.

Best regards
@lscharmer

return new Transformation\ToStringTransformation();
}

public function toSvg(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name toSvg says nothing about a QR code (same with the corresponding ToSvgTransformation class).
Please rename the method and transformation (maybe something like toSvgQRCode or so).

namespace ILIAS\Refinery\URI;

use ILIAS\Refinery\Transformation;
use ILIAS\Refinery\Transformation as ITransformation;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use ILIAS\Refinery\Transformation as ITransformation;
use ILIAS\Refinery\Transformation;

use ILIAS\Refinery\Constraint;
use ILIAS\Refinery\Transformation;
use ILIAS\Refinery\String\Encoding\Group as EncodingGroup;
use ILIAS\Data\QR\ErrorCorrectionLevel;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use ILIAS\Data\QR\ErrorCorrectionLevel;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file improvement php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants